Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add TLS endpoint to kepler exporter #1899

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aharivel
Copy link
Contributor

Add TLS support via the new web configuration file, following the Prometheus Exporter Toolkit style for consistency across exporters.

  • Usage:

kepler --web.config.file=web-config.yml

  • Content of web-config.yml:

tls_server_config:
cert_file: /path/to/server.crt
key_file: /path/to/server.key

Add TLS support via the new web configuration file, following the
Prometheus Exporter Toolkit style for consistency across exporters.

* Usage:

kepler --web.config.file=web-config.yml

* Content of web-config.yml:

tls_server_config:
  cert_file: /path/to/server.crt
  key_file: /path/to/server.key

Signed-off-by: Anthony Harivel <[email protected]>
Copy link
Contributor

github-actions bot commented Dec 20, 2024

🤖 SeineSailor

Here is a concise summary of the pull request changes:

Summary: This pull request adds TLS support to the Kepler exporter by introducing a new web-config.yml file for specifying TLS server certificates and keys. The main function now reads TLS configuration from the file, enabling the server to listen on an HTTPS endpoint when configured.

Key Modifications:

  • Added web-config.yml file for TLS configuration
  • Updated main function to read TLS configuration from the file
  • Server now supports TLS and listens on HTTPS endpoint when configured

Impact: The external interface remains unchanged, but the code behavior changes when TLS is configured, switching from HTTP to HTTPS.

Observations/Suggestions:

  • It would be beneficial to include tests to verify the TLS configuration and HTTPS endpoint functionality.
  • Consider adding documentation to explain the new TLS configuration options and their usage.
  • Review the error handling and logging mechanisms to ensure they are adequate for TLS-related issues.

Copy link
Collaborator

@SamYuan1990 SamYuan1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://prometheus.io/docs/introduction/roadmap/#tls-and-authentication-in-http-serving-endpoints I don't mind for TLS endpoint, but considering with prometheus TLS note for official exporters...

TLS and authentication in HTTP serving endpoints

TLS and authentication are currently being rolled out to the Prometheus, Alertmanager, and the official exporters. Adding this support will make it easier for people to deploy Prometheus components securely without requiring a reverse proxy to add those features externally.

at meanwhile, if we are going to enable TLS for kepler, considering in a k8s cluster with node A,B,C
what will be the hostname in cert for the TLS cert?what will be the changes for kepler pod?
as for example if we are going to use kepler.svc.nodeX.com as host name in k8s cluster, and the key for each node are different.
so, somehow will it impact/it will change our deployment yaml?
as we have to inject/put a specific node cert-key pair as configuration?

mark with change request for more information and discussion.

@aharivel
Copy link
Contributor Author

Hi @SamYuan1990 ,
I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?

While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:

Node Exporter Configuration
Node Exporter Setup Tasks

Furthermore, they are also using Kepler, as shown here:
Kepler Integration Tasks

This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?

Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?

Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

@SamYuan1990
Copy link
Collaborator

Hi @SamYuan1990 , I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?

While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:

Node Exporter Configuration Node Exporter Setup Tasks Furthermore, they are also using Kepler, as shown here: Kepler Integration Tasks

This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?

Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?

Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

From security point of view, yes. We can make kepler protected by TLS to improve our baseline for sensitive data protection.
Once more, this PR is a great job and I just "mark with change request for more information and discussion."

So from deployment point of view, considering with kepler community operator and kepler community helm chat as deployment approaches.

  • do we have issue open on kepler operator or kepler helm chat with TLS support?
  • are we going to integrate with cert mgr or sidecar as istio? by document supporting or we just leave it to user?
  • what if kepler running as a static pod?

and btw, any impacts to kepler RPM service?

from security point:

what's specific level for kepler's sensitive data? what are they?

Additionally, Kepler is handling sensitive data.

how to protect those data according to it's level? to clear/clarify our security boundary.

  • do we need considering a key/a cert with password?
  • do we recommended user to enable tls verify when enable tls?
  • do we need considering mtls?

for instance:
if we considering pod list is a sensitive data, and which should not listed by plaintext through network, as kubelet having TLS to protect those info. we should add TLS to protect those info as well. I suppose this is from node mgr's points of view.
if we don't want user A to see user B's pod on the kepler metric... should we considering username and password? I suppose this is from user's point of view.

from impl points of view:

is there any chance for us to reuse https://github.com/prometheus/exporter-toolkit
ref
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/README.mdplain=1#L383
reuse https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md
https://github.com/prometheus/exporter-toolkit/tree/master/web
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/go.mod#L28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants